fix(pi): allow known read-only T3 tools without approval by StiensWout · Pull Request #17852 · pingdotgg/t3code

StiensWout · 2026-10-10T15:40:35Z

Problem

Supervised Pi sessions asked for approval even for canonical T3 tools marked read-only.

Change

Use the T3 server annotation only when the active tool belongs to the injected HTTP bridge. Mutations, unannotated tools, replacement extensions and other MCP servers still require confirmation.

Scope and approval

Maintainer-requested Pi support follow-up. Scope is approval policy for canonical T3 tools annotated read-only.

Verification

Validation: 22 bridge/injection tests, Pi typecheck and scoped lint/format checks passed.

_{Prepared for Wout by gpt-6.1-sol in Codex.}

macroscopeapp · 2026-10-10T15:42:49Z

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The change alters the production approval boundary by allowing annotated T3 MCP calls to bypass confirmation across supervised Pi modes. Despite focused tests and source-path checks, this permission-policy and security-sensitive behavior change warrants human review.

^{You can add or adjust custom eligibility rules. Learn more.}

coderabbitai · 2026-10-10T15:48:03Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.config.ts
Review profile: CHILL
Plan: Advanced
Run ID: ee284867-7eaa-4cc6-8a65-6fcd1320d524

📥 Commits

Reviewing files that changed from the base of the PR and between ac48563 and 23f514f.

📒 Files selected for processing (3)

packages/provider-pi/src/server/mcpBridge.testkit.ts
packages/provider-pi/src/server/mcpExtensionSource.test.ts
packages/provider-pi/src/server/mcpExtensionSource.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

The Pi MCP launch environment now receives the configured extension path. The approval hook uses read-only annotations and registered tool source paths to decide whether a tool requires approval.

Changes

Pi MCP Read-Only Approval

Layer / File(s)	Summary
Pass the extension path to Pi `packages/provider-pi/src/server/mcpExtensionSource.ts`, `packages/provider-pi/src/server/mcpInjection.ts`, `packages/provider-pi/src/server/mcpInjection.test.ts`	The generated extension source includes the extension-path environment variable. `buildPiRpcLaunch` removes an inherited value and sets the configured path when the extension is enabled. Tests check the launch environment.
Check read-only tools against their source path `packages/provider-pi/src/server/mcpExtensionSource.ts`, `packages/provider-pi/src/server/mcpExtensionSource.test.ts`, `packages/provider-pi/src/server/mcpBridge.testkit.ts`	The extension tracks catalog tools marked read-only. The approval hook skips approval only when the tool name, read-only status, and registered source path match. Tests cover runtime modes, T3 tool-name prefixes, and tools from other paths or namespaces.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant PiLaunch as buildPiRpcLaunch
  participant Extension as T3 MCP extension
  participant Approval as Pi approval hook
  PiLaunch->>Extension: Set configured extension path
  Extension->>Approval: Register annotated tool and source path
  Approval->>Approval: Check tool name, annotation, and source path

Merge Risk | ⚪ Minimal · up to 23f51

Merge Risk: ⚪ Minimal · up to 23f51

The approval change is mergeable after normal checks; no specific blocking issue is established by the supplied evidence.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 23f51

Automatic approval is restricted by both read-only metadata and T3 extension provenance. Existing credentials and server-side authentication remain in place. No introduced bypass was established, but tool-replacement behavior in supported Pi versions remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The exemption applies to every eligible canonical catalog entry, not only capability discovery. Read-only declarations include environment metadata and project thread listings, so accessible data need not belong solely to the calling thread. The PR changes confirmation requirements without minting credentials or broadening their existing authority.

Trust Boundaries and Controls

observed — The new exemption requires a tracked read-only name and matching normalized extension provenance. Missing provenance or getAllTools support fails the exemption. Launch construction removes inherited provenance, and tests demonstrate confirmation for replacement source paths, unrelated prefixes, and non-read-only annotations.
observed — Skipping local confirmation does not skip HTTP authentication. The existing MCP middleware resolves the presented bearer credential for each request and supplies its invocation scope. Server-side handler authority checks remain separate from the extension's annotation-based decision.

Resilience and Maintainability Implications

inferred — The snapshot now carries approval authority, making annotation stability security-relevant. Inspected canonical tools use static annotations and fixed server-layer registration, which counters the hypothetical stale-annotation transition. A live server replacement or changing catalog contract was not established and should not be treated as a verified flaw.

Hardening Proposals

proposed — Validate the provenance-to-dispatch contract against supported Pi versions, including simultaneous same-name registrations and partial registration failures. If uniqueness is not guaranteed, bind exemptions to the dispatch-selected registration rather than any matching entry. This addresses an assurance gap, not a demonstrated exploit.

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: allowing known read-only T3 tools to run without approval.
Description check		The description includes Problem, Change, Scope and approval, and Verification sections. It accurately describes the approval-policy change and lists focused validation. The scope section does not inc…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

@StiensWout

## What's Changed * fix(pi): preserve tool images and structured results by @StiensWout in pingdotgg/t3code#17851 * fix(server): Claude 5 task lists reach the tasks drawer by @Mnigos in pingdotgg/t3code#14964 * fix(web): find bar and thread details panel stop covering each other by @MatthewFeroz in pingdotgg/t3code#17858 * fix(web): use server metadata for file chip icons by @Yash-Singh1 in pingdotgg/t3code#17923 * fix(desktop): copy images from HTML previews by @Bil0000 in pingdotgg/t3code#17555 * docs(pi): update installation and remote login guidance by @StiensWout in pingdotgg/t3code#17836 * fix(pi): preserve native abort outcomes by @StiensWout in pingdotgg/t3code#17853 * fix(pi): keep thinking defaults specific to each model by @StiensWout in pingdotgg/t3code#17835 * fix(pi): preserve shell command exit codes by @StiensWout in pingdotgg/t3code#17834 * fix(pi): expire and cancel extension approvals by @StiensWout in pingdotgg/t3code#17840 * feat(pi): include native sessions in usage reports by @StiensWout in pingdotgg/t3code#17848 * fix(server): route Copilot ACP subagent output into subagent threads by @maria-rcks in pingdotgg/t3code#17714 * fix(web): composer banner titles truncate beside their icon instead of wrapping by @maria-rcks in pingdotgg/t3code#17699 * fix(server): Muse turns no longer fail on Windows by @ntindle in pingdotgg/t3code#17163 * fix(pi): allow known read-only T3 tools without approval by @StiensWout in pingdotgg/t3code#17852 * fix: worktree threads keep their worktree when the agent starts, and messages sent during setup queue by @maria-rcks in pingdotgg/t3code#17654 * fix(server): keep Claude workflows alive while they report progress by @maria-rcks in pingdotgg/t3code#17715 * fix(web): media preview centers its content and pins the close button by @maria-rcks in pingdotgg/t3code#17951 * fix(server): threads without a project no longer need Git installed by @t3dotgg in pingdotgg/t3code#17959 * fix(web): toggling tools and thinking at the bottom keeps you at the bottom by @t3dotgg in pingdotgg/t3code#17954 * fix(web): Compact chip follows Claude's real prompt cache TTL by @t3dotgg in pingdotgg/t3code#17945 * fix(usage): bound OpenCode history reads to prevent backend OOM by @Yash-Singh1 in pingdotgg/t3code#17961 * refactor: format diff line counts through one shared helper by @maria-rcks in pingdotgg/t3code#17948 * fix: new projects start their first thread in the project folder, not a worktree by @t3dotgg in pingdotgg/t3code#17371 ## New Contributors * @ntindle made their first contribution in pingdotgg/t3code#17163 **Full Changelog**: pingdotgg/t3code@v0.0.46-nightly.20261010.2948...v0.0.46-nightly.20261011.2955 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.46-nightly.20261011.2955

@StiensWout

## What's Changed * fix(pi): preserve tool images and structured results by @StiensWout in pingdotgg/t3code#17851 * fix(server): Claude 5 task lists reach the tasks drawer by @Mnigos in pingdotgg/t3code#14964 * fix(web): find bar and thread details panel stop covering each other by @MatthewFeroz in pingdotgg/t3code#17858 * fix(web): use server metadata for file chip icons by @Yash-Singh1 in pingdotgg/t3code#17923 * fix(desktop): copy images from HTML previews by @Bil0000 in pingdotgg/t3code#17555 * docs(pi): update installation and remote login guidance by @StiensWout in pingdotgg/t3code#17836 * fix(pi): preserve native abort outcomes by @StiensWout in pingdotgg/t3code#17853 * fix(pi): keep thinking defaults specific to each model by @StiensWout in pingdotgg/t3code#17835 * fix(pi): preserve shell command exit codes by @StiensWout in pingdotgg/t3code#17834 * fix(pi): expire and cancel extension approvals by @StiensWout in pingdotgg/t3code#17840 * feat(pi): include native sessions in usage reports by @StiensWout in pingdotgg/t3code#17848 * fix(server): route Copilot ACP subagent output into subagent threads by @maria-rcks in pingdotgg/t3code#17714 * fix(web): composer banner titles truncate beside their icon instead of wrapping by @maria-rcks in pingdotgg/t3code#17699 * fix(server): Muse turns no longer fail on Windows by @ntindle in pingdotgg/t3code#17163 * fix(pi): allow known read-only T3 tools without approval by @StiensWout in pingdotgg/t3code#17852 * fix: worktree threads keep their worktree when the agent starts, and messages sent during setup queue by @maria-rcks in pingdotgg/t3code#17654 * fix(server): keep Claude workflows alive while they report progress by @maria-rcks in pingdotgg/t3code#17715 * fix(web): media preview centers its content and pins the close button by @maria-rcks in pingdotgg/t3code#17951 * fix(server): threads without a project no longer need Git installed by @t3dotgg in pingdotgg/t3code#17959 * fix(web): toggling tools and thinking at the bottom keeps you at the bottom by @t3dotgg in pingdotgg/t3code#17954 * fix(web): Compact chip follows Claude's real prompt cache TTL by @t3dotgg in pingdotgg/t3code#17945 * fix(usage): bound OpenCode history reads to prevent backend OOM by @Yash-Singh1 in pingdotgg/t3code#17961 * refactor: format diff line counts through one shared helper by @maria-rcks in pingdotgg/t3code#17948 * fix: new projects start their first thread in the project folder, not a worktree by @t3dotgg in pingdotgg/t3code#17371 ## New Contributors * @ntindle made their first contribution in pingdotgg/t3code#17163 **Full Changelog**: pingdotgg/t3code@v0.0.46-nightly.20261010.2948...v0.0.46-nightly.20261011.2955 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.46-nightly.20261011.2955

fix(pi): allow known read-only T3 tools without approval

ac48563

github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 10, 2026

coderabbitai Bot approved these changes Oct 10, 2026

View reviewed changes

fix(pi): resolve conflicts with main

23f514f

github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Oct 10, 2026

Yash-Singh1 merged commit 5f7294d into pingdotgg:main Oct 10, 2026
30 checks passed

Navigation Menu

Repository navigation

fix(pi): allow known read-only T3 tools without approval - #17852

fix(pi): allow known read-only T3 tools without approval#17852
Yash-Singh1 merged 2 commits into
pingdotgg:mainfrom
StiensWout:t3code/pi-readonly-mcp

StiensWout commented Oct 10, 2026

Uh oh!

macroscopeapp Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

StiensWout commented Oct 10, 2026

Problem

Change

Scope and approval

Verification

Uh oh!

macroscopeapp Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approvability

Uh oh!

coderabbitai Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

macroscopeapp Bot commented Oct 10, 2026 •

edited

Loading

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading